Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Deposit Transaction RLP encoding #75

Merged
merged 2 commits into from
Jun 27, 2023

Conversation

pcw109550
Copy link
Member

@pcw109550 pcw109550 commented Jun 23, 2023

There was no proper RLP encoding for encoding body. Also deposit transactions related using eth protocol was also not properly RLP encoded.

This resulted in database sync failure; p2p sync between two op-erigon clients. To be more specific, after GET_BLOCK_BODIES_66 is sent, the receiver creates outbound messages having type BLOCK_BODIES_66 which contains body of specific block. The body was empty because the block had only deposit transactions, and deposit transaction was not marshaled, resulting in decoding failure on sender side.

Added proper RLP encoding logic for deposit transactions.

Added block_test.go::TestCanEncodeAndDecodeBodyTransactions test for checking RLP encoding logic for each transaction type including deposit transactions.

@pcw109550 pcw109550 self-assigned this Jun 23, 2023
@pcw109550 pcw109550 changed the title Handle Deposit Transsaction RLP encoding Handle Deposit Transaction RLP encoding Jun 23, 2023
@pcw109550 pcw109550 requested a review from ImTei June 26, 2023 09:56
@ImTei
Copy link
Member

ImTei commented Jun 27, 2023

I don't think we need to handle deposit transactions of p2p transactions messages, because deposit tx will not be in the tx pool. however, handling it seems to be good for consistency.

@pcw109550 pcw109550 merged commit 8d59181 into op-erigon Jun 27, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants